change(firestore): close Firestore instance more gracefully when "Clear Site Data" button pressed in browser #9118
Conversation
…ce() from another tab.
🦋 Changeset detectedLatest commit: 3f24c15 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (208,932 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
| request.onsuccess = (event: Event) => { | ||
| let error: unknown; | ||
| if (clearSiteDataEvent[0]) { |
There was a problem hiding this comment.
the c++ side of my brain hurts when i see this :)
TS playground suggests it's safe to do, but why not use if (clearSiteDataEvent.length > 0) { ?
There was a problem hiding this comment.
What, you don't like the looks of accessing uninitialized memory? ;)
My memory told me that the TypeScript compiler was failing to infer that clearSiteDataEvent[0] is defined when the if statement checked clearSiteDataEvent.length > 0; however, I just tested it out and it appears to compile just fine. So, I've changed if to check for a non-zero length.
| // "onupgradeneeded" event listener. Do this because throwing from the | ||
| // "onupgradeneeded" event listener results in a generic error being | ||
| // reported to the "onerror" event listener that cannot be distinguished | ||
| // from other errors. |
There was a problem hiding this comment.
Thanks for the comment. I presume onsuccess always gets called after onupgradeneeded ?
There was a problem hiding this comment.
Yes, that is correct: onsuccess is called after onupgradeneeded (as long as onupgradeneeded doesn't throw an exception)
… of `clearSiteDataEvent[0]`
This PR is the 2nd in a chain of PRs (previous PR: #9087) that unify the logic that handles unexpected IndexedDB database deletion.
This PR enhances the robustness and user experience when the Firestore SDK's IndexedDB database is unexpectedly deleted, such as by a user clicking 'Clear Site Data' in their browser or another tab clearing persistence. It introduces a more granular event system for database deletion notifications, enabling the SDK to provide more informative error messages and ensure a more graceful termination process.
Highlights
ClearSiteDataDatabaseDeletedEvent,VersionChangeDatabaseDeletedEvent) to differentiate between various IndexedDB database deletion scenarios, providing more context to theDatabaseDeletedListener.setDatabaseDeletedListenerto receive detailed event information, allowing for more specific logging and error handling during Firestore termination.FirestoreErrorof typefailed-preconditionis now thrown. This error provides a clear message to the user about potential IndexedDB corruption and suggests a page reload, and is thrown after attempting to terminate Firestore.SimpleDb.open()to defer handling of 'Clear Site Data' detection from theonupgradeneededevent to theonsuccessevent. This change prevents generic errors and allows for more precise and actionable error reporting to thedatabaseDeletedListener.